Skip to content

fix(plugin-reports): forward the caller's execution envelope to the report read (#7204) - #7283

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7204-report-envelope-forwarding
Aug 10, 2026
Merged

fix(plugin-reports): forward the caller's execution envelope to the report read (#7204)#7283
os-help merged 1 commit into
mainfrom
claude/issue-7204-report-envelope-forwarding

Conversation

@os-help

@os-help os-help commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7204

executeReport rebuilt a five-field projection of the caller's ExecutionContext (userId / tenantId / positions / permissions / isSystem) before handing it to the engine read that produces the report — while the method's own comment promised "reports execute with the caller's identity". The projection is what broke that promise.

Premise: verified on origin/main @ 1da1f32a9

The card's report-service.ts:466 anchor is the isSystem line of the projection; PR #7206's annotation widening is in the base, so the parameter is already the full ExecutionContext and the whole-envelope forward typechecks with no signature change. The engine consumer is at packages/objectql/src/engine.ts:2247 (accessible_org_ids under the group posture), with hasTz two lines up at :2228.

Repro first — measured, on row sets

Real ObjectKernel + real ObjectQL engine + @objectstack/driver-sql on better-sqlite3 :memory:, group posture, one caller whose accessible_org_ids spans org_a + org_b, three rows split across both:

before after
interactive engine.find ['a1','a2','b1'] ['a1','a2','b1']
saved report run() ['a1','a2'] ['a1','a2','b1']
ad-hoc runAdHoc() ['a1','a2'] ['a1','a2','b1']
scheduled CSV digest 2 data rows 3 data rows

buildDriverOptions reads accessible_org_ids by name to widen the driver's native tenant scope to the membership union (ADR-0105 D2 / #3623); absent, drivers "fall back to equality: fail toward isolation". So the direction is under-reporting, not a leak — a correctness defect, silent, with nothing in the output saying rows were dropped.

timezone was dropped by the same projection, and it has a read-path consumer too: applyFormulaPlan (engine.ts:777, called from the find path at :6007) evaluates read-time formula fields with execCtx.timezone. Pinned on the row's VALUE — at 2026-08-10T20:00:00Z a today() formula field resolved 2026-08-10 in the report and 2026-08-11 interactively for a Pacific/Kiritimati caller. (buildDriverOptions' own hasTz use is write-path only in driver-sql — autonumber {YYYYMMDD} fill — so the formula path is where a read observes it.)

The change

The read receives the caller's envelope whole — the #6206 ruling / #6523: enforcement adjudicates on the whole resolveAuthzContext envelope, never a per-site subset — minus the __-prefixed keys plugin-security stamps for the operation in flight, and as a fresh object. Same shape plugin-audit (#7141 / PR #7143) and service-storage (#7145 / PR #7207) landed.

On the PM's mechanism hypothesis — whether the in-place-mutation hazard from #7141 applies here: it does, in a different direction, so the __ strip stays. The report's operation object IS report.object_name, so plugin-security resolving the depth for it is correct. Two hazards remain and the strip covers both:

  • Stale inbound depth. The route hands over the request envelope, which the middleware may have already written into for a different object earlier in the same request, and it only OVERWRITES __readScope when it resolves permission sets for the new object (if (permissionSets.length > 0), security-plugin.ts:1157). A stale depth would otherwise survive into a question it was never resolved for.
  • Write-back. sc.__readScope = … mutates whatever object it is handed, so forwarding the caller's own envelope by reference would leave the report's depth on the request context the route goes on using.

Also preserved deliberately:

  • the projection's positions / permissions / isSystem defaults, byte-for-byte, so this change adds fields without changing any that were already there (the isSystem ?? false fallback the card flagged);
  • assertExportAllowed still runs against the un-projected caller context, above and untouched — pinned by identity (toBe(caller)).

Reverse verification — two directions, and every pin goes red in exactly one

(a) fix removed (git checkout origin/main -- report-service.ts): 5 red / 3 green.

× a SAVED report ...      → expected [ 'a1', 'a2' ] to deeply equal [ 'a1', 'a2', 'b1' ]
× an AD-HOC report ...    → expected [ 'a1', 'a2' ] to deeply equal [ 'a1', 'a2', 'b1' ]
× a SCHEDULED run ...     → expected [ …(2) ] to have a length of 3 but got 2
× rows the ACTIVE org has none of → expected [] to deeply equal [ 'b1' ]
× forwards the business timezone  → expected '2026-08-10' to be '2026-08-11'
✓ an EMPTY accessible set still collapses to active-org equality
✓ isolated posture: the report stays at active-org equality
✓ no posture provider: equality, never widened

The three green ones are the point: group is the only posture the widening applies to, so a change that turned them red would have traded under-reporting for exposure.

(b) the naive context: context — the direction the card asked to be tested rather than assumed. It fixes the row sets and breaks the three preservation pins instead:

✓ forwards the principal fields the projection used to drop
× does NOT forward the middleware-private `__` keys → expected [ '__readScope', …(3) ] to deeply equal []
× hands the engine a FRESH object → expected { … } not to be { … } // Object.is equality
× keeps the projection's defaults → expected { userId: 'u1' } to match object { userId: 'u1', positions: [], …(2) }

So no pin in the block is decoration: (a) names the defect, (b) names what the naive fix would have cost.

Tests

packages/plugins/plugin-reports/src/report-group-posture-scope.integration.test.ts (new, 8 cases) carries the end-to-end row-set claims against the real stack — deliberately not a fake engine, because "the key is on the context object" is exactly what this defect looked like from inside the service. The structural half (which keys cross, freshness, defaults, the export-axis input) is 6 new cases in report-service.test.ts against the existing fake engine.

@objectstack/objectql + @objectstack/driver-sql are added as devDependencies of plugin-reports for that harness; runtime deps are unchanged.

Local verification

pnpm --filter @objectstack/plugin-reports typecheck   # clean
pnpm --filter @objectstack/plugin-reports test        # 3 files, 68 tests passed
pnpm --filter @objectstack/plugin-reports build       # DTS + CJS + ESM ok
npx eslint packages/plugins/plugin-reports/src --max-warnings=0   # clean
pnpm check:org-identifier          # OK (1733 files)
pnpm check:nul-bytes               # OK (6640 files)
pnpm check:slot-lookup             # ratchet holds, none new
pnpm check:query-options-erasure   # ratchet holds, none new
pnpm check:tenant-chokepoint       # OK
pnpm check:authz-resolver          # OK
pnpm check:empty-changeset         # OK
pnpm check:published-files         # OK
pnpm check:type-check-coverage     # OK

Changeset: patch for @objectstack/plugin-reports, naming the before/after.


Generated by Claude Code

…eport read (#7204)

`executeReport` rebuilt a five-field projection of the caller's
`ExecutionContext` (`userId` / `tenantId` / `positions` / `permissions` /
`isSystem`) before handing it to the engine read that produces the report,
while the method's own comment promised "reports execute with the caller's
identity". `accessible_org_ids` was not in that projection, and
`buildDriverOptions` reads it by name (ADR-0105 D2 / #3623) to widen the
driver's native tenant scope to the caller's membership union under the
`group` tenancy posture; absent, drivers fall back to active-org equality.
So the identical query returned the union interactively and collapsed to the
active org inside a saved or scheduled report -- silently short rows, no error.

Measured end-to-end on a real kernel + `@objectstack/driver-sql` sqlite
`:memory:`: 3 rows across two member orgs came back as `['a1','a2','b1']`
interactively and `['a1','a2']` in the report; the scheduled CSV digest
emailed the owner the same two.

The read now receives the envelope whole (the #6206 ruling / #6523), minus the
`__`-prefixed keys plugin-security stamps for the operation in flight, as a
fresh object so a callee's stamp cannot write back into the caller's request
context -- the shape plugin-audit (#7141) and service-storage (#7145) landed.
`timezone` comes with it, so a read-time formula field resolves the caller's
calendar day. The projection's `positions` / `permissions` / `isSystem`
defaults are kept byte-for-byte.

Direction outside `group` is unchanged and pinned: `isolated`, no posture
provider, and a `group` caller with an empty accessible set all still read at
active-org equality.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 4:50am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-reports.

2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via @objectstack/plugin-reports)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-reports)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants